Skip to content

FFI: Generic getter(s) for X.509 objects - #5188

Merged
reneme merged 3 commits into
randombit:masterfrom
Rohde-Schwarz:feature/generic_ffi_getters
Feb 26, 2026
Merged

FFI: Generic getter(s) for X.509 objects#5188
reneme merged 3 commits into
randombit:masterfrom
Rohde-Schwarz:feature/generic_ffi_getters

Conversation

@reneme

@reneme reneme commented Dec 22, 2025

Copy link
Copy Markdown
Collaborator

We're currently working on extending the strongswan botan plugin to parse and verify X.509 certificates and CRLs using Botan. That use case requires adding a number of new getters to fetch detailed information from certificates and CRLs, many of which are duplicated for both types like the raw signature bits, the serial number or the (optional) authority key ID.

Instead of extending the FFI with several new functions (including the duplication for botan_x509_cert_t and botan_x509_crl_t) we would like to propose a more generic API that can work with both types and provide access to a number of (shared) data fields, like so:

int copy_to_buffer(void*, uint8_t* bin, size_t); // some binary view function
int copy_to_string(void*, uint8_t* bin, size_t); // some string view function

botan_x509_cert_t cert = /* ... */;
botan_x509_crl_t crl   = /* ... */;

uint8_t cert_akid[32];
uint8_t crl_akid[32];
size_t outlen = 32;
botan_x509_cert_get_binary_values(cert, BOTAN_X509_AUTHORITY_KEY_ID, 0, cert_akid, copy_to_buffer);
botan_x509_crl_get_binary_values(crl , BOTAN_X509_AUTHORITY_KEY_ID, 0, crl_akid, copy_to_buffer);

char uri[128];
int rc = BOTAN_FFI_SUCCESS;
for (size_t i = 0; rc != BOTAN_FFI_ERROR_OUT_OF_RANGE; ++i) {
   rc = botan_x509_cert_get_string_values(cert, BOTAN_X509_OCSP_RESPONDER_URLS, i, uri, copy_to_string);
}
/* ... use the AKIDs somehow */

Our hope is that this significantly reduces the boilerplate and API surface while also being easily extensible by amending the "type enum" in an ABI-compatible way. Note that the proposed API allows to access multi-value fields using a simple index parameter.

Currently, the following data fields can be retrieved via this API:

Data field Data Type Certificate CRL
TBS data binary x x
Signature Bits binary x x
Encoded Signature Scheme binary x x
Encoded Subject DN binary x
Encoded Issuer DN binary x x
Subject Key ID binary (x)
Authority Key ID binary (x) (x)
Serial Number binary x x
Public Key w/ Algorithm Identifier binary x
DER Encoding binary x x
PEM Encoding string x x
CRL distribution points multi-value string (x)
OCSP responders multi-value string (x)
CA issuers URLs multi-value string (x)

@reneme
reneme requested a review from randombit December 22, 2025 15:33
@reneme reneme self-assigned this Dec 22, 2025
@reneme reneme added the enhancement Enhancement or new feature label Dec 22, 2025
@randombit

Copy link
Copy Markdown
Owner

I'm not firmly against it. I would definitely suggest view-style functions since, with cases like this where the lengths can be somewhat open-ended (especially so given the set of fields might be expanded over time), retry handling is kind of a pain.

OTOH if we have a single kind of getter dispatch function, is there really that much duplication to have 2 instead, one for certs and one for CRLs? One thing I did try for in the FFI interface is making it a type safe as possible when used from C. (This stems from the fact that a major driver of the FFI interface as it exists today is due to the requirements of RNP, which at the time was written entirely in C). This function which will require a cast for either input type bypasses that a bit, and tbh I don't really see how 2 type specific getter-dispatchers would be a problem, admittedly I'm not at all familiar with strongswan internals.

@reneme

reneme commented Dec 23, 2025

Copy link
Copy Markdown
Collaborator Author

I would definitely suggest view-style functions since, with cases like this where the lengths can be somewhat open-ended [...].

👍

OTOH if we have a single kind of getter dispatch function, is there really that much duplication to have 2 instead, one for certs and one for CRLs?

No, we could split it up. Perhaps the boilerplate might grow a bit faster once we introduce additional getters for strings, lists and such. Mostly I was interested in exploring the ergonomics of such a unified API. That said, I think I'll leave it in until the work on the strongswan plugin converges. Reverting this won't be a lot of work anyway.

@reneme
reneme force-pushed the feature/generic_ffi_getters branch 3 times, most recently from f81c4fb to 28c62c5 Compare December 23, 2025 09:24
@coveralls

coveralls commented Dec 23, 2025

Copy link
Copy Markdown

Coverage Status

coverage: 90.409% (+0.02%) from 90.393%
when pulling 9d9388a on Rohde-Schwarz:feature/generic_ffi_getters
into 81beaa2 on randombit:master.

Comment thread src/lib/ffi/ffi.h Outdated
@reneme
reneme force-pushed the feature/generic_ffi_getters branch 2 times, most recently from 0324a33 to 08c8fae Compare January 5, 2026 15:46
@reneme
reneme marked this pull request as ready for review January 5, 2026 15:46
@reneme

reneme commented Jan 5, 2026

Copy link
Copy Markdown
Collaborator Author

In my opinion this is now ready-for-review. Documentation as well as PR description is updated accordingly.
We're heavily relying on this API within the upcoming strongSwan extension and it allows quite ergonomic usage.

OTOH if we have a single kind of getter dispatch function, is there really that much duplication to have 2 instead, one for certs and one for CRLs?

I split the functions into CRL/cert as proposed. The required overhead indeed does not justify the added complexity in the API contract as well as the underlying implementation of the polymorphy.

The potential extension discussed in an inline thread would make a lot of sense, in my opinion. Some details may need further discussion though, so I would suggest to split this off into another PR.

@reneme
reneme force-pushed the feature/generic_ffi_getters branch from 08c8fae to 7dacff2 Compare January 5, 2026 15:55
Comment thread src/lib/ffi/ffi.h
@arckoor

arckoor commented Jan 5, 2026

Copy link
Copy Markdown
Contributor

Could you increase the FFI version? Would be nice if it could be the intended release date like outlined here. Also any plans for an impl for the python binding, or should that happen at a later date by someone(tm)? :p

@reneme

reneme commented Jan 5, 2026

Copy link
Copy Markdown
Collaborator Author

Also any plans for an impl for the python binding, or should that happen at a later date by someone(tm)?

Shouldn't be too hard to do. I would leave it to a future step though, once the API in this is finalized.

This comment was marked as resolved.

@randombit randombit left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks looking good

Comment thread src/lib/ffi/ffi_cert.cpp Outdated
Comment thread src/lib/ffi/ffi_cert.cpp Outdated
@reneme
reneme force-pushed the feature/generic_ffi_getters branch from 9824817 to 6b8cb78 Compare January 11, 2026 18:50
Comment thread src/lib/ffi/ffi_cert.cpp
Comment thread src/lib/ffi/ffi_cert.cpp Outdated
Comment thread src/lib/ffi/ffi_cert.cpp Outdated
@reneme

This comment was marked as resolved.

@reneme
reneme force-pushed the feature/generic_ffi_getters branch from 6b8cb78 to 73ba696 Compare January 12, 2026 16:10
@reneme

reneme commented Jan 12, 2026

Copy link
Copy Markdown
Collaborator Author

I've added *_count() functions to the generic getters as suggested and addressed the latest review comments.

Re: *_count() - their implementations are maximally trivial for now, basically just actually "counting" how many entries are produced by the enumerator functions where BOTAN_FFI_ERROR_NO_VALUE counts as "0". Given that (1) the enumerators mostly just invoke the viewer functions straight on the C++ objects' memory and (2) long lists are not to-be-expected for X.509 certificates, I believe this is just good enough for now. It definitely has the nice side effect that the *_count() functions aren't at any risk to become inconsistent with the enumerator implementations.

This comment was marked as resolved.

This comment was marked as resolved.

@reneme
reneme force-pushed the feature/generic_ffi_getters branch 3 times, most recently from a6a6401 to 9d9388a Compare January 14, 2026 06:31
@randombit

Copy link
Copy Markdown
Owner

@reneme Shit sorry I thought this had already landed on master. Looks goods to me, needs a rebase though.

@reneme
reneme force-pushed the feature/generic_ffi_getters branch from 9d9388a to dc8b17a Compare February 25, 2026 14:30
Namely:
  * botan_x509_cert_view_binary_values
  * botan_x509_cert_view_string_values
  * botan_x509_crl_view_binary_values
  * botan_x509_crl_view_string_values

This allows fetching standard information from X.509 objects
such as the serial number of both a certificate and a CRL but
also more specific information like OCSP responder URLs.

Some getter types may have more than one value. For such cases,
the getters have an index parameter to enumerate all elements.
These functions are currently implemented by trivially counting the
number of elements their associated enumerator functions are actually
producing. Given that most values are anyway viewed straight from the
C++ objects' memory, the overhead should be negligible. The main
advantage of this approach is its minimal maintenance burden.
@reneme
reneme force-pushed the feature/generic_ffi_getters branch from dc8b17a to 525e34c Compare February 25, 2026 14:36
@reneme

reneme commented Feb 25, 2026

Copy link
Copy Markdown
Collaborator Author

I thought this had already landed on master.

... actually, me too. We're still waiting on another team to test run this in their system and it fully disappeared from my short term memory. Thanks for noticing and moving it forward.

I applied your suggestion re:"assert unreachable" and rebased to master (testing locally that it compiles/unittests with --disable-modules=x509 as well).

@reneme

reneme commented Feb 25, 2026

Copy link
Copy Markdown
Collaborator Author

@randombit This is now rebased and ready to go. I can haz ✅ plz?

@randombit randombit left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🍏

@reneme
reneme merged commit 27fd567 into randombit:master Feb 26, 2026
45 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Enhancement or new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants